Added into_vec method for Matrix #3
Conversation
src/base/matrix.rs
Outdated
| @@ -4718,23 +4718,26 @@ impl<T> super::alias::Matrix1<T> { | |||
| } | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Contribution starts here
|
Can you please explain why did you need to duplicate code? Please format your answer so it is easily readable. |
Deleted duplicated code
|
I modified the code so that there is no more duplicated text. The problem was that I copied the whole code from local to github instead of only 21 lines. |
src/base/matrix.rs
Outdated
| unsafe { | ||
| resulted_vector.push(self.get_unchecked((i, j)).clone()); | ||
| } |
There was a problem hiding this comment.
| unsafe { | |
| resulted_vector.push(self.get_unchecked((i, j)).clone()); | |
| } | |
| resulted_vector.push(unsafe { self.get_unchecked((i, j)) }.clone()); |
Please explain why did you use unsafe and why it is safe to do so.
There was a problem hiding this comment.
As the method suggests (get_unchecked), there is no warranty that (i, j) are in bounds, that the operation does not fail or that the operation is memory safe. Now, the methods called by resulted_vector are not unsafe. If the resulted_vector is immutable, then it will result in compilation error, not that the operation is not memory safe. If the push exceeds vector capacity, then Rust is designed to reallocate space. I will modify.
There was a problem hiding this comment.
I observed that was the way transpose method access the elements of a matrix in matrix.rs so I wanted to maintain the format.
There was a problem hiding this comment.
Please add a comment in the code explaining why this is used and why it is safe.
There was a problem hiding this comment.
I've added two comments that should clarify the problem
Lines 2361-4720 are mistakenly duplicated, because of copying at the end the whole code from VS instead of only 4721-4740. So, matrix.rs has the code (matrix.rs + matrix.rs + into_vec method) instead of (matrix.rs + into_vec method). Now, regarding into_vec method, it was created so that the project code pattern is respected. The structure implementation is similiar in definition to the implementation that embeds hash function (choosing the storage type etc.). The two loops on the matrix and the elements access are similiar to the transpose matrix function (to maintain coerence), code that existed before in the open source. A new idea that this contribution introduces is that the code is flexible and permits adding multiple algorithms for into_vec method, as stated in the documentation ///, last commit (that didn't exist before in this project). In the contribution, only the pragmatic method that transforms a matrix into a vector by concatenating the rows is implemented, but leaves room for new algorithms (examples: matrix -> vector by concatenating columns. If the matrix is squared, then the developer can create concatenation of parallel diagonals, or whatever is needed. The applications of this idea of multiple algorithms can be taken as an open problem, because for some algorithms there are no immediate or evident applications). This contribution solves an open issue regarding into_vec method. The final code is from the last commit (2 weeks ago) that edits some coding style aspects and introduces the idea previously mentioned, last 21 lines.